Skip to content

[WIP] Feat/cloudflare#8

Merged
leomp12 merged 20 commits into
ecomplus:masterfrom
confere-stores:feat/cloudflare
Feb 3, 2022
Merged

[WIP] Feat/cloudflare#8
leomp12 merged 20 commits into
ecomplus:masterfrom
confere-stores:feat/cloudflare

Conversation

@Mazurco066
Copy link
Copy Markdown
Contributor

Raw file

Mazurco066 added a commit to confere-stores/storage-api that referenced this pull request Feb 1, 2022
ecomplus#6

* feat: creating cloudflare adapter

* feat: cloudflare integration

* feat: cloudflare integration

* feat: cloudflare integration

* feat: cloudflare integration

* fix: ecom review ecomplus#1

* fix: ecom review ecomplus#1

* fix: ecom review ecomplus#3

* fix: ecom review ecomplus#4

* fix: ecom review ecomplus#4

* fix: ecom review ecomplus#5

* fix: ecom review ecomplus#6

* fix: ecom review ecomplus#7

* fix: ecom review ecomplus#8
Comment thread bin/web.js Outdated
@leomp12 leomp12 changed the title Feat/cloudflare [WIP] Feat/cloudflare Feb 1, 2022
Comment thread bin/web.js
const respond = () => {
logger.log(`${storeId} ${key} ${bucket} All optimizations done`)
res.json({ bucket, key, uri, picture })
key += `${v4()}-${filename}`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key += `${v4()}-${filename}`
key += `${Date.now()}-${filename}`

acho que o uuid só pra isso aqui é exagero, principalmente porque ainda concatena com o filename original..

Comment thread bin/web.js
Comment on lines +26 to +27
// UUID Generator
const { v4 } = require('uuid')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// UUID Generator
const { v4 } = require('uuid')

Comment thread bin/web.js
const respond = (suc = true) => {
if (suc) {
logger.log(`${req.storeId} ${key} ${bucket} All optimizations done`)
res.json({ bucket, key, uri: picture['zoom'] ? picture['zoom'].url : '' , picture })
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

na verdade se a zoom não existir teremos problemas haha

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aí a resposta não poderia ser 200

Comment thread lib/Cloudflare.js
const label = labels[labels.length - 1]

// Download image, zoom must be jpeg
download(vari, { 'Accept': label === 'zoom' ? 'image/jpeg,image/*,*/*;q=0.8' : 'image/webp,image/*,*/*;q=0.8' }, (err, imageBody) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
download(vari, { 'Accept': label === 'zoom' ? 'image/jpeg,image/*,*/*;q=0.8' : 'image/webp,image/*,*/*;q=0.8' }, (err, imageBody) => {
download(vari, { 'Accept': label === 'zoom' ? 'image/png,image/jpeg,image/*,*/*;q=1' : 'image/webp,image/*,*/*;q=0.8' }, (err, imageBody) => {

imagem zoom em qualidade 100%

Comment thread lib/Cloudflare.js
setTimeout(() => cloudflareClient.delete(`/images/v1/${id}`), 60000)
// Download the correct variations
variants.forEach(vari => {
if (vari.includes('normal') || vari.includes('zoom') || vari.includes('big') || vari.includes('w90')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problema é só que jogando pra cá você não considera mais o que vem do config

Comment thread lib/Cloudflare.js
// Download image, zoom must be jpeg
download(vari, { 'Accept': label === 'zoom' ? 'image/jpeg,image/*,*/*;q=0.8' : 'image/webp,image/*,*/*;q=0.8' }, (err, imageBody) => {
processed++
if (err) logger.error(err)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

não deveria encerrar o loop no erro?

Comment thread bin/web.js
Comment on lines +295 to +296
const contentType = label === 'zoom' ? 'image/jpeg' : 'image/webp'
const fileFormat = label === 'zoom' ? 'jpg' : 'webp'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ué mas a imagem original não necessariamente é jpg na verdade

Comment thread bin/web.js
Comment on lines +313 to +314
// async handle with callback URL
redisClient.setex(genRedisKey(id, true), 600, JSON.stringify(s3Options))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isso aqui era só pra callback do cloudinary pra manipulação assíncrona na verdade, o cloudflare não envia esse webhook então isso não tem utilidade mais..

@leomp12
Copy link
Copy Markdown
Member

leomp12 commented Feb 3, 2022

@Mazurco066 como tá no seu fork fica ruim pra eu editar umas coisas antes do merge, aí apesar dos comentários já vou dar merge e fixar os minors em outro commit em seguida...
valeu man 👍🏾 😃

@leomp12 leomp12 merged commit 9b0ac5f into ecomplus:master Feb 3, 2022
@leomp12
Copy link
Copy Markdown
Member

leomp12 commented Feb 3, 2022

a25b8cf upload da zoom diretamente no s3, mantendo mimetype, busca as apenas versões otimizadas (conforme configuração) no cloudflare...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants